Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reimplement BOM export tool #8756

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

SchrodingersGat
Copy link
Member

Ref: #8685 (comment)

Followup to #8685

@SchrodingersGat SchrodingersGat added this to the 1.0.0 milestone Dec 24, 2024
Copy link

netlify bot commented Dec 24, 2024

Deploy Preview for inventree-web-pui-preview canceled.

Name Link
🔨 Latest commit 2ab91d1
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/6780d7dd0c25800008a2d610

Copy link

codecov bot commented Dec 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.55%. Comparing base (c99aae5) to head (2ab91d1).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8756      +/-   ##
==========================================
+ Coverage   85.23%   85.55%   +0.31%     
==========================================
  Files        1177     1177              
  Lines       51804    51549     -255     
  Branches     2094     2094              
==========================================
- Hits        44156    44101      -55     
+ Misses       7119     6919     -200     
  Partials      529      529              
Flag Coverage Δ
backend 87.38% <100.00%> (+0.40%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@matmair
Copy link
Member

matmair commented Dec 24, 2024

@SchrodingersGat this will require an API version bump as it removes a number of endpoints; maybe it makes more sense to include these in #8686

@SchrodingersGat
Copy link
Member Author

@matmair good point, I will roll the BOM exporter into this one.

I just have to devise a clean way to have a "custom" exporter within our new framework

@SchrodingersGat
Copy link
Member Author

@matmair @wolflu05 I have an idea here which requires some further fleshing out - feedback welcomed :)

Current Situation

The new "data export" framework basically takes our existing DRF serializers, and pushes the data into a formatted file for download. It is very efficient, low code overhead, and integrates seamlessly with the PUI data tables. The intent here is that the table data (as presented and filtered) is basically "what you get" when you download the data via the table interface.

Legacy BOM Exporter

The legacy BOM exporter contained a lot of extra information. Supplier parts, alternative part options, pricing data, stock information, etc. That's all fine, but it doesn't fit nicely within the concept of a simple "press this button to download the table data" approach.

Custom Report

I see the "BOM exporter" (as it existed in CUI) working better as a custom report generator. There are custom operations in the background which do not take place in the regular "export serialized data" framework.

So, what I would like to propose is that we extend the "report generation" framework to allow arbitrary file generation. While currently it is limited to templated PDF files, there's no reason we can't have a "BOM export" plugin which generates an excel or .csv file for download.

However this would require a lot of ripping up / refactoring of existing code. Also, how do we handle custom reports that do not make use of the report "template"?

Before I go and start making changes, I'd love some feedback from you guys on how you think it should work - or potentially other ideas / approaches.

@SchrodingersGat SchrodingersGat changed the title Cleanup Reimplement BOM export tool Dec 29, 2024
@SchrodingersGat SchrodingersGat added the import / export Data importing, exporting and processing label Dec 29, 2024
@matmair
Copy link
Member

matmair commented Dec 29, 2024

I think the cleanest way would be to add a custom serializer that handles the BOM export and its options. We could add the option to use plugins to add serializers for custom exports.
Using the report framework does not seem intuitive - it is really oriented around templates right now and rewriting it would either break APIs for no apparent benefit or make testing really hard. IMO this most makes sense as a new tool and endpoint, not in the report framework.

@SchrodingersGat
Copy link
Member Author

Thanks for the feedback, that sounds pretty reasonable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
import / export Data importing, exporting and processing refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants